Implement MigratableKVStore for all KV Stores#945
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
benthecarman
force-pushed
the
migrate-dbs
branch
from
653afa3 to
0708bfe
Compare
benthecarman
force-pushed
the
migrate-dbs
branch
from
0708bfe to
3953744
Compare
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Thanks, first pass.
Would be cool to add a commit that does a full end-to-end test of migration. I.e., pick a random backend, setup a node with it, open a channel to another node, send some payments, then drop everything and migrate to a random new backend, then ensure the node is still fully functional.
| && secondary_namespace.is_empty() | ||
| && key == VSS_SCHEMA_VERSION_KEY | ||
| { | ||
| continue; |
There was a problem hiding this comment.
Hmm, maybe we should attempt to always migrate to the latest schema version, allowing nodes to upgrade?
There was a problem hiding this comment.
Here we are just listing all keys so we need to support both. The actual migration will just do whatever happens on .write(). Assuming it'll be a fresh VSS store it should persist as the latest schema version
benthecarman
force-pushed
the
migrate-dbs
branch
from
3953744 to
23e2840
Compare
This was unimplemented for the sqlite kv store. Useful if the user wants to migrate to a different database and also in tests so we don't have to re-init and setup a node. AI-assisted-by: OpenAI Codex
This was unimplemented for the postgres kv store. Useful if the user wants to migrate to a different database and also in tests so we don't have to re-init and setup a node. AI-assisted-by: OpenAI Codex
Extract the existing obfuscated key selection so later VSS listing changes can reuse it without changing the parsing behavior. AI-assisted-by: OpenAI Codex
This was unimplemented for the VSS kv store. Useful if the user wants to migrate to a different database and keeps VSS aligned with the other persistent stores. AI-assisted-by: OpenAI Codex
This was unimplemented for the in-memory kv store. Useful in tests so we can migrate data across all supported store implementations. AI-assisted-by: OpenAI Codex
Creates a node with LN and on-chain state and then randomly migrates through all the KV store options and checks it still has its state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
benthecarman
force-pushed
the
migrate-dbs
branch
from
23e2840 to
ff5c0a5
Compare
| use lightning_persister::fs_store::v2::FilesystemStoreV2; | ||
| use rand::seq::SliceRandom; | ||
|
|
||
| async fn drop_tables<'a>(table_names: impl IntoIterator<Item = &'a String>) { |
There was a problem hiding this comment.
Do we really need all this verbose boilerplate for the single test? Seems just choosing two backends at random could be only a few LoC inline?
|
|
||
| [dev-dependencies] | ||
| lightning = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "3dfcc4cca1866c5e5d4d4eaf3b82e09584e2ce5c", features = ["std", "_test_utils"] } | ||
| lightning-persister = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "3dfcc4cca1866c5e5d4d4eaf3b82e09584e2ce5c", features = ["tokio"] } |
There was a problem hiding this comment.
Why do we need an extra dev-dependency now?
3 participants
Pulled out of #915
These were unimplemented for our kv stores. Useful if the user wants to migrate to a different database and also in tests so we don't have to re-init and setup a node.